Skip to content

fix(verify): route embedded checks to controller - #1046

Open
RerankerGuo wants to merge 1 commit into
agentscope-ai:mainfrom
RerankerGuo:fix/verify-embedded-routing
Open

fix(verify): route embedded checks to controller#1046
RerankerGuo wants to merge 1 commit into
agentscope-ai:mainfrom
RerankerGuo:fix/verify-embedded-routing

Conversation

@RerankerGuo

Copy link
Copy Markdown
Contributor

Summary

  • detect the embedded controller separately from the Manager container
  • run MinIO and Matrix localhost checks in the controller while keeping Agent health checks in the Manager
  • discover custom gateway/console host ports from Docker or Podman mappings when container env does not expose them
  • preserve the legacy single-container fallback and add a recording regression for both topologies

Root cause

The post-install verifier still assumed all services shared the Manager network namespace. Current embedded installs run Tuwunel and MinIO inside agentteams-controller and create agentteams-manager as a separate container on agentteams-net, so probing the Manager at 127.0.0.1:9000 and :6167 reports false failures. The controller also does not receive custom host-port values as environment variables, making the hard-coded port defaults inaccurate for customized installs.

Validation

  • regression on origin/main: embedded topology reports 4/6 while legacy reports 6/6
  • install/tests/test-verify-container-routing.sh: embedded and legacy assertions both pass after the fix, including custom 19080/19001 mappings
  • bash -n install/hiclaw-verify.sh install/tests/test-verify-container-routing.sh
  • shellcheck -S warning install/hiclaw-verify.sh install/tests/test-verify-container-routing.sh
  • git diff --check

Overlap check

PR #999 only adds a Dashboard probe, and #569 only adds a Codex Agent-health branch. Neither changes the embedded infrastructure-container routing or mapped-port discovery addressed here.

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction-2 / mgr=openclaw / wk=openclaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction-2 / mgr=copaw / wk=copaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (controller-cr-2 / mgr=copaw / wk=copaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (controller-cr / mgr=copaw / wk=hermes)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction / mgr=copaw / wk=copaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction / mgr=copaw / wk=hermes)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction / mgr=openclaw / wk=openclaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (controller-cr / mgr=openclaw / wk=openclaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (controller-cr / mgr=copaw / wk=copaw)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction-2 / mgr=copaw / wk=hermes)

Commit: 43501c8
Workflow run: #1670

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@RerankerGuo

Copy link
Copy Markdown
Contributor Author

CI attribution for run 29483183894: translation, change detection, the controller/base build, and all six downstream image builds passed.

I audited the raw archive for all 10 failed integration jobs. Every job has exactly one empty AGENTTEAMS_LLM_API_KEY, exits from install-embedded with LLM API Key is required, and records no test-start marker. There are zero hiclaw-verify invocations, so the changed verifier never ran. This is the same base-workflow bootstrap issue addressed by #1026; no branch-specific code change is warranted.

The focused local routing regression remains green for both embedded and legacy topologies (2 passed, 0 failed).

@shiyiyue1102

Copy link
Copy Markdown
Collaborator

Thanks for the detailed CI attribution. I reviewed the verifier routing and reran the focused regression locally: both embedded and legacy routing cases pass. The implementation matches the current embedded topology by keeping infrastructure checks in the controller container and the Agent health check in the Manager container.

The remaining blocker is that the required integration checks on the current head are still recorded as failed, even though the archived jobs appear to have exited before the tests because the LLM key was empty. Please update or rebase the branch onto the latest main and rerun the required checks so the PR has a clean, current CI result. Once that is green, this should be ready for final review.

@EricZHANG1688

Copy link
Copy Markdown

Thanks for the fix — exactly the symptom I was hitting.

I'd like to add a host-side access perspective as a downstream application developer:

After deploying the latest image locally, I noticed that no host-side tool can reach Matrix 6167 directly. I built a two-account round-trip verification (Alice sends via Tuwunel → Bob reads from the same room via a different access_token), and it works inside the container via docker exec, but there's no way for an external client to connect from the host.

Repro on host (after a fresh install/hiclaw-install.sh deploy):

# Inside the container — works
$ docker exec hiclaw-controller curl -s -o /dev/null -w "%{http_code}\n" \
    http://127.0.0.1:6167/_matrix/client/versions
200

# From the host — fails
$ curl -s -o /dev/null -w "%{http_code}\n" --max-time 3 \
    http://localhost:6167/_matrix/client/versions
000

# docker ps confirms 6167 is not published
$ docker ps --filter name=hiclaw-controller --format '{{.Ports}}'
127.0.0.1:18001->8001/tcp, 127.0.0.1:8099->8080/tcp, 127.0.0.1:18088->8088/tcp
# 6167 missing

I traced install/hiclaw-install.sh and found that only four host ports are published: 18080/18001/18088/18888. 6167 is intentionally internal. The Helm tuwunel-service.yaml is type: ClusterIP, so it is also unreachable from outside the cluster.

This is complementary to #1046's fix direction — #1046 addresses the intra-cluster "where to run the localhost check" problem; my concern is the cross-cluster "host users cannot reach Matrix at all" problem. They don't conflict, but the documentation gap is real either way.

Suggested options, in order of preference:

  1. Default-publish 6167 (best, with AGENTTEAMS_MATRIX_EXPOSE_PORT env var for opt-in/opt-out, matching least-privilege)
  2. Document the workaround in README: "host users can verify Matrix via docker exec <container> curl http://127.0.0.1:6167"
  3. Provide a bridge subcommand: agentteams matrix-port-forward [--port 6167] for host-side tools

I have a 7-check reproduction script covering HTTP probes, TCP socket probes, docker inspect IP routing, port mapping comparison, and docker-compose.yml static scan. Happy to share the full script or a minimal gist if useful.

Thanks again for the responsive maintenance on the embedded topology!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants